Skip to content

Conversation

@VeraZab
Copy link
Contributor

@VeraZab VeraZab commented Jul 19, 2018

new color picker
ex

demo app in react-colorscales project
screen shot 2018-07-24 at 3 34 17 pm

@VeraZab VeraZab force-pushed the adjust-colorscalepicker branch from 1c223d9 to f1e9d21 Compare July 19, 2018 21:08
@VeraZab VeraZab changed the title WIP Colorpicker adjustments Colorpicker adjustments Jul 19, 2018
@VeraZab VeraZab requested review from dmt0 and nicolaskruchten July 19, 2018 21:09
dev/App.js Outdated
import 'brace/mode/json';
import 'brace/theme/textmate';
import {categoryLayout, traceTypes, chartCategory} from 'lib/traceTypes';
import 'brace/theme/textmate'; //
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty comment...


return (
<Field {...this.props}>
<Field {...adjustedProps}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you just do <Field {...this.props} label={false}>?
I think if a props is defined twice, only the last one counts, so you can overwrite with an empty.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's nice, didn't think of that one thanks!

@dmt0
Copy link
Contributor

dmt0 commented Jul 20, 2018

ok, didn't play with it, but code-wise - 💃

@VeraZab
Copy link
Contributor Author

VeraZab commented Jul 20, 2018

Hey @jackparmer I just updated the react-colorscales project page, to include all api changes:
https://github.com/plotly/react-colorscales

I changed the look of the colorPicker to adapt to small UIs, but I have only extended the api, so it should be ok for the projects using it.

I've put up a few screenshots above, and deployed to gh-pages:
https://plotly.github.io/react-colorscales/

Let me know if you'd like any adjustments :)

@jackparmer
Copy link
Contributor

Hey, cool! I love it!

Only nit that caught me eye is Cmocean should be all lowercase (cmocean). That seems to be how it's presented in the literature: https://matplotlib.org/cmocean/

image

@nicolaskruchten
Copy link
Contributor

I'm a bit confused by the UX of the new picker here...

image

There's no clear indication of what the active scale is, or of how to "close" the picker, or why "reset" is there... I think I like this layout but I preferred the previous setup where the active scale is always present above, and the panel opens when you're fiddling with it and it updates the activate scale, and then you can close the panel when you're done fiddling.

@VeraZab VeraZab force-pushed the adjust-colorscalepicker branch from a317df4 to b716024 Compare July 24, 2018 19:31
@VeraZab
Copy link
Contributor Author

VeraZab commented Jul 24, 2018

ex

ok I corrected titles, they're small caps now, just for a consistent look.
There's also the 'scale' word in the label to make what we're selecting clearer.

@nicolaskruchten , I added back the toggle that you liked and on another UX pass, as you said, we could think of a more discoverable way to make these controls open/close.

@VeraZab VeraZab force-pushed the adjust-colorscalepicker branch 2 times, most recently from 27f3196 to a3159e1 Compare July 24, 2018 19:55
COLOR_PICKER_CONSTANTS.COLORSCALE_DESCRIPTIONS[selectedColorscaleType];
const colorscaleOptions = COLOR_PICKER_CONSTANTS.COLORSCALE_TYPES.map(
type => ({
label: type + ' scale',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually now that I look at it, could we make this scales with an s please? :)

super();

this.state = {
selectedColorscaleType: 'sequential',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: in a group of traces, this default makes less sense, and should be overrideable to categorical

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup, will look at how to select the initial state better in a subsequent pr..
but the structure is there to add it easily now :)

@VeraZab
Copy link
Contributor Author

VeraZab commented Jul 24, 2018

ok @nicolaskruchten did the latest color/swatch number modifications
screen shot 2018-07-24 at 5 50 53 pm

@VeraZab VeraZab force-pushed the adjust-colorscalepicker branch from 3c856d0 to 5d34d91 Compare July 24, 2018 21:55
@nicolaskruchten
Copy link
Contributor

💃

@VeraZab VeraZab force-pushed the adjust-colorscalepicker branch from 5d34d91 to e860bb2 Compare July 25, 2018 12:53
@VeraZab VeraZab merged commit b27271d into master Jul 25, 2018
@VeraZab VeraZab deleted the adjust-colorscalepicker branch July 25, 2018 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants